Skip to content

fix: support model prefix in with_dimensions/with_measures after joins#207

Open
boringdata wants to merge 3 commits intomainfrom
fix/model-prefix-with-dimensions
Open

fix: support model prefix in with_dimensions/with_measures after joins#207
boringdata wants to merge 3 commits intomainfrom
fix/model-prefix-with-dimensions

Conversation

@boringdata
Copy link
Owner

Summary

Test plan

  • test_model_prefix_in_with_dimensionst.carriers.name works in with_dimensions after join
  • All existing join prefixing tests pass
  • All existing deferred API tests pass

🤖 Generated with Claude Code

boringdata and others added 2 commits March 9, 2026 14:32
Add prefix-aware column navigation to ColumnScope and MeasureScope so
that dimension/measure lambdas can use model prefixes (e.g.,
`t.flights.carrier`) after joins. Previously this only worked in
`.filter()`. Also adds _DimensionTableProxy for dimension evaluation
with merged dimension map awareness.

Closes #136

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lent fallback

Proxy classes (_ColumnPrefixProxy, _DimPrefixProxy) now raise AttributeError
when a prefixed name isn't found, preventing silent resolution to unrelated
unprefixed columns. Also preserves AttributeError from proxy evaluation in
Dimension.__call__ for proper dependency resolution.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@boringdata
Copy link
Owner Author

Codex Review Summary

Codex (gpt-5.3-codex) reviewed this PR and found 2 P1 issues related to silent proxy fallbacks. Both have been fixed in commit bb07a6f:

Issues Found & Fixed

  • [P1] _ColumnPrefixProxy.__getattr__ (measure_scope.py:34-38): Was falling back to getattr(self._tbl, name) when prefixed column not found, which could silently resolve t.customers.id to unprefixed t.id. Fixed: Now raises AttributeError with available prefixed columns listed.

  • [P1] _DimPrefixProxy.__getattr__ (ops.py:862-866): Same silent fallback issue for dimension lookups. Fixed: Now raises AttributeError listing available prefixed dimensions.

  • [P2] Dimension.__call__ proxy error swallowing (ops.py:918-919): Retry path was catching all exceptions from proxy evaluation, masking dependency resolution errors. Fixed: Now re-raises AttributeError from proxy for proper dependency chain detection.

All 103 tests passing after fixes.

boringdata added a commit that referenced this pull request Mar 11, 2026
# Conflicts:
#	src/boring_semantic_layer/tests/test_yaml.py
hussainsultan pushed a commit that referenced this pull request Mar 12, 2026
…211)

* feat: support calculated_measures and .all() in YAML config

Add `calculated_measures` section to YAML schema for derived metrics
that reference other measures by name. Expressions are evaluated at
runtime against MeasureScope, which also enables `.all()` for
percent-of-total window aggregation patterns.

Closes #202, closes #115

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: support model prefix in with_dimensions/with_measures after joins

Add prefix-aware column navigation to ColumnScope and MeasureScope so
that dimension/measure lambdas can use model prefixes (e.g.,
`t.flights.carrier`) after joins. Previously this only worked in
`.filter()`. Also adds _DimensionTableProxy for dimension evaluation
with merged dimension map awareness.

Closes #136

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: raise on invalid prefixed column/dimension lookups instead of silent fallback

Proxy classes (_ColumnPrefixProxy, _DimPrefixProxy) now raise AttributeError
when a prefixed name isn't found, preventing silent resolution to unrelated
unprefixed columns. Also preserves AttributeError from proxy evaluation in
Dimension.__call__ for proper dependency resolution.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: keep helpful dimension error formatting with proxy fallback

---------

Co-authored-by: boringdata <boringdata@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant